-
-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed typo for AssertIncludes cop. Changed includes -> include #19
Fixed typo for AssertIncludes cop. Changed includes -> include #19
Conversation
The change fixes a false negative. Can you please add the changelog entry? |
18aaef6
to
a1b39c3
Compare
@koic : Added the changelog. Can you please have a look? |
CHANGELOG.md
Outdated
@@ -2,6 +2,10 @@ | |||
|
|||
## master (unreleased) | |||
|
|||
### Bug fixes | |||
|
|||
* [#19](https://github.com/rubocop-hq/rubocop-minitest/pull/19): Fixes typo `collection.includes?` to `collection.include?` in `Minitest/AssertIncludes` cop. ([@abhaynikam][]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to write a false negative correction rather than a typo :-) The following is an example.
Fix a false negative for `Minitest/AssertIncludes` when using `include` method in arguments of `assert` method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @koic 🎉 . Updated the changelog
a1b39c3
to
620fd3a
Compare
Btw, I wonder if we should check for both for the sake of Rails users. I know the ActionSupport aliases are quite popular there. @koic any thoughts? |
It seems my understanding is lacking 💦 Which Active Support API is that? |
Hmm, that's weird - I can't find it here https://edgeguides.rubyonrails.org/active_support_core_extensions.html#extensions-to-enumerable I could swear that back in the day Rails shipped some aliases like |
@bbatsov Thanks for the answer. AFAIK, there is an Other methods cannot be remembered 😅 First of all, I think that it is better to support only |
Yeah, I agree. |
This PR fixes the typo for AssertIncludes cop. Changes
collection.includes
->collection.include
at all places.